Add batch actions (purge, query orchestrations/entities)#111
Add batch actions (purge, query orchestrations/entities)#111andystaples wants to merge 16 commits intomainfrom
Conversation
|
@copilot Can you open a PR to these changes that adds the necessary tests to the newly-added test files covering an acceptable set of use cases for each of the newly added actions? Please update the |
|
@andystaples I've opened a new pull request, #112, to work on those changes. Once the pull request is ready, I'll request review from you. |
There was a problem hiding this comment.
Pull request overview
This PR adds batch operation support for managing orchestrations and entities, enabling querying and purging of multiple instances at once. The changes introduce new client methods for batch queries and purges, along with supporting infrastructure and tests.
Changes:
- Added batch query methods (
get_all_orchestration_states,get_orchestration_state_by,get_all_entities,get_entities_by) to retrieve multiple orchestration and entity instances with filtering capabilities - Added batch purge method (
purge_orchestrations_by) to delete multiple orchestration instances based on filters - Enhanced existing
purge_orchestrationmethod to return aPurgeInstancesResultwith information about deleted instances - Refactored orchestration state parsing logic to support both single-instance and batch queries
- Added deprecation to
EntityMetadata.from_entity_responsein favor offrom_entity_metadata - Added helper function
get_int_valuefor wrapping optional integers in protobuf messages
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/durabletask/test_batch_actions.py | New test file for batch orchestration query operations using the TaskHubGrpcClient |
| tests/durabletask-azuremanaged/test_dts_batch_actions.py | New test file for batch orchestration query operations using DurableTaskSchedulerClient with support for fetching inputs/outputs |
| durabletask/internal/helpers.py | Added get_int_value helper function for wrapping optional integers in protobuf Int32Value |
| durabletask/entities/entity_metadata.py | Deprecated from_entity_response method and refactored to use new from_entity_metadata method for better separation of concerns |
| durabletask/client.py | Added batch query/purge methods for orchestrations and entities, refactored state parsing, updated existing methods to use helper functions, and added PurgeInstancesResult class |
Comments suppressed due to low confidence (1)
durabletask/entities/entity_metadata.py:3
- The
deprecateddecorator from thewarningsmodule was introduced in Python 3.13, but this project requires Python 3.10+ (as specified in pyproject.toml). This will cause an ImportError on Python 3.10, 3.11, and 3.12. Consider using an alternative deprecation approach such aswarnings.warn(DeprecationWarning(...))inside the function, or a third-party library that supports older Python versions.
from durabletask.entities.entity_instance_id import EntityInstanceId
* Initial plan * Add comprehensive batch action tests and clean_entity_storage method Co-authored-by: andystaples <77818326+andystaples@users.noreply.github.com> * Merge base branch and resolve conflicts Co-authored-by: andystaples <77818326+andystaples@users.noreply.github.com> * Address PR review comments on test improvements Co-authored-by: andystaples <77818326+andystaples@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: andystaples <77818326+andystaples@users.noreply.github.com>
There was a problem hiding this comment.
Reviewers - I'm not sure these tests are worth adding, tbh. If we ever implement these gRPC endpoints into the Go sidecar, our tests will immediately break. Any objection to just commenting them out for now, and relying on the DTS tests?
There was a problem hiding this comment.
Right now, it looks like they're just a reminder for us to write tests once they are implemented. However, I'm confused why they rely on the go sidecar since we're in the python SDK? If they can break from a PR outside of this repo, I think they become less useful.
If this is truly just a test for the go sidecar, I would rather we have those in the go repo. Can you expand on this a little bit?
There was a problem hiding this comment.
The durabletask (non-DTS) tests use the Go sidecar as the emulator for the durability layer, just like how the DTS tests use the DTS emulator docker image, yes, it's testing two things but AFAIK, and this was before my time, it was the easiest way to set up E2E for this project.
I'll go ahead and remove these tests, with a note to add them when possible
durabletask/client.py
Outdated
| fetch_inputs_and_outputs=fetch_inputs_and_outputs | ||
| ) | ||
|
|
||
| def get_orchestration_state_by(self, |
There was a problem hiding this comment.
That's a strange method name - is this what we use in other SDKs?
There was a problem hiding this comment.
At least in .NET, we just have 1 method which takes an optional filter.
There was a problem hiding this comment.
This matches the Funtions Python SDK, but probably better to match the other portable SDKs and have the translation layer for Functions + durabletask python. Will update this in future commit
There was a problem hiding this comment.
Updated to match .NET - takes a Filter instead of a list of params, and there is one method for each. Also removed _continuation_token from the public API
halspang
left a comment
There was a problem hiding this comment.
A nit and a question to go along with the other comments.
durabletask/client.py
Outdated
| ) -> List[OrchestrationState]: | ||
| if max_instance_count is None: | ||
| # Some backends do not behave well with max_instance_count = None, so we set to max 32-bit signed value | ||
| max_instance_count = (1 << 31) - 1 |
There was a problem hiding this comment.
Nit: Is there no int.max in python?
There was a problem hiding this comment.
No, int values are unbounded in python
Adds batch actions for orchestration/entity management